Codecov Report

❌ Patch coverage is 70.74286% with 256 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

API preview: https://pr-20733-api.vm6.ai

Code Review: PR #20733

Summary

Reviewed the runner/guest session-history sidecar cache implementation across 5 commits. The sidecar is modeled as an optional workspace-image-cache optimization, not as a new source of truth. The code validates metadata identity and body file identity before use, verifies hash/size before restore, accounts sidecar bytes in cache GC/inspection, and falls back to remote session history on cache failure.

Key Findings

Critical Issues (P0)

None found.

High Priority (P1)

None found.

Testing Review

Coverage

• perf: cache verified final session-history bytes beside promoted workspace images -> covered by crates/runner/src/workspace_image_cache/tests/mod.rs, crates/runner/src/workspace_promotion/tests.rs, and guest helper coverage in crates/guest-agent/src/session_history_identity.rs.
• perf: restore from local sidecar on fresh workspace-image hits -> covered by crates/runner/src/executor/tests/agent_run_tests.rs.
• fix: verify workspace sidecar content before restore -> covered by corrupted sidecar fallback and Codex zstd restore tests in crates/runner/src/executor/tests/agent_run_tests.rs.
• fix: prune stale/invalid sidecars and surface prune errors -> covered by crates/runner/src/workspace_image_cache/tests/mod.rs.
• fix: clean sidecar export temp files -> covered by workspace promotion tests and reviewed cleanup paths.

Convention Compliance

No testing convention violations found. Rust tests use real temp directories/filesystem operations, httpmock for external HTTP, and existing sandbox-mock at the sandbox boundary. No fake timers, active sleeps, internal TS mocks, direct fetch mocks, or filesystem mocks were introduced.

Testing Verdict: Adequate

Bad Smell Analysis

• Mock violations: 0
• Direct fetch mocking: 0
• Filesystem mocking: 0
• Fake timers / artificial sleeps: 0
• Type suppressions / lint suppressions: 0
• Production unwrap / panic: 0
• Dynamic imports / TypeScript any: 0
• Defensive fallback misuse: 0

Verification

Reviewed and ran:

git diff --check origin/main...HEAD
cargo test --manifest-path crates/Cargo.toml -p runner session_history_sidecar -- --nocapture
cargo test --manifest-path crates/Cargo.toml -p runner workspace_sidecar -- --nocapture
cargo test --manifest-path crates/Cargo.toml -p runner codex_zstd_sidecar -- --nocapture
cargo clippy --manifest-path crates/Cargo.toml -p runner -p guest-agent --all-targets -- -D warnings
cargo fmt --manifest-path crates/Cargo.toml --all -- --check

All passed.

Recommendations

No changes requested.

Verdict

LGTM.

Full review details: codereviews/20260709/
Testing standards: docs/testing.md